-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#4549 kotlin ijar test cases #4632
Conversation
Can one of the admins verify this patch? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@cushon Could you give this a look as well?
mkdir -p $3 || fail "could not ensure output dir" | ||
${IJAR} $1 $2 || fail "ijar failed" | ||
pushd $3 | ||
${JAR} xf $2 || fail "could not extract jar contents" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please just indent with 4 spaces here, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's showing up as 4 spaces on my side (not a tab).
inline fun inheritedInlineFun(op: () -> Unit) { op() } | ||
} | ||
|
||
object ObjectInheritingInline: AbstractClassWithInline() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add newline at the end of the file.
@@ -0,0 +1,3 @@ | |||
#!/bin/bash -eu | |||
|
|||
kotlinc -module-name inline-cases -d inline-cases.jar InlineCases.kt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add newline at the end of the file.
updated so the test correctly fails.
fe3044b
to
70d453c
Compare
@hsyed : Do you have any updates? Do you still want to merge this PR? |
@philwo , could you please take another look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't know much about Kotlin nor ijar, but this seems like a good addition to the test suite to me.
Thanks @philwo for the update! Do you want to import this? |
@laszlocsomor Yes, I'll send you a CL. :) |
It is ready to merge |
@hsyed Sorry for the delay. Philipp needs to make some changes internally so that he can import this PR. |
@hsyed I tried to import this PR, but the tests don't pass. I see the following errors. Can you help me figuring out what's wrong? You can see the current version of the code that I tried to check in here: https://bazel-review.googlesource.com/c/bazel/+/44791 (please ignore any diffs in files except third_party/ijar/... - they are unfortunately generated by our importing script and are just a UI issue)
|
@hsyed Friendly ping - I would love to get this merged. |
Hi All, Bazel sheriff is here. What is the status of this? |
This isn't in scope any longer. I'll close it. |
Inlining test cases for #4549. The jar also contains the metadata file which needs to be retained
META-INF/inline-cases.kotlin_module